Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade "stop action" in profiler command #2613

Merged
merged 15 commits into from
Aug 18, 2023

Conversation

Winson-Huang
Copy link
Contributor

@Winson-Huang Winson-Huang commented Aug 12, 2023

升级 stop action,适配其支持的选项

stop action 主要涉及测试结果的输出,需处理 profiler.sh 中 FIEL、OUTPUT、FORMAT 三个变量对应的选项。在 sh 脚本中分析结果允许输出到标准输出或文件,且与输出内容格式是完全解耦的。Arthas 的设计可能是:要求必须将分析结果保存到文件,因此在 processStop 方法中只要判断 file 选项为空就会指定一个默认文件名。

目前对 file 选项的捕捉本身没有问题,可以对应到 sh 中的 FILE 变量。

OUTPUT 变量:

在 sh 中通过 -o 选项转换为此变量,但目前 arthas 中没有该选项,仅有一个相关的 format 选项,它仅用于构造默认文件名的后缀(未传递给 execute 方法)且只有两个可选值,【分析结果】的内容格式是让 async-profiler 通过这个后缀名推断的。在 sh 中,通过设置 OUTPUT 可以得到其他格式的输出(如 flat 等格式)。要想在 profiler command 中支持这些格式,可以将 format 选项的值也用于构造 executeArgs,将其与 sh 的 -o 选项对应起来。

根据 sh 脚本中的使用说明,-o 选项可选值为 flat|traces|collapsed|flamegraph|tree|jfr,且均可在 arguments.cpp 中找到详细解释,其中 flat 和 traces 可加 [=N] 格式参数,在 Arthas 中,用 format 选项捕捉这些值,并为选项添加短名称 o。本选项的值直接拼接到 JVM TI 格式参数中,与 file 选项不同,在 Arthas 中需要注意 executeArgs 方法构造 JVMTI 格式字符串的过程。

在需要生成默认文件名时,不直接使用 format 选项的值而是添加一层判断:先判断 format 是否有效,若无效就让后缀取 html(或其他值),然后根据 format 的各种情况确定后缀名,然后再生成文件名。

format 选项不要设定默认值,否则当用户不指定 format 但指定 file 时,用 file 后缀名推断输出格式(async profiler 内部会处理这一过程)会失效。例如,假设 format 默认值为 flamegraph,用户指定了 file 后缀为 jfr 但未指定 format,用户预期是输出 jfr 格式,但 flamegraph 会被传递给 async profiler,因此将输出火焰图格式(虽然后缀名是 jfr)。

FORMAT 变量:

shell 脚本中对此变量的处理分散在多处,但都位于 L149 开始的循环块中。L184 -s。L187 -g。L190 -a。L193 -l。L200 -I

L208 --filter 选项暂不实现,未出现在 shell 脚本说明中,其参数格式不明确。

L213 --title 需要转义 XML 特殊字符,以及逗号(防止与 JVM TI 冲突)。

L219 仅实现 --minwidth 即可。L223 --reverse。L226 仅需要 --total。

相关 issue

issue #2164

@hengyunabc
Copy link
Collaborator

这些新增加的选项,需要在文件里增加使用案例,不然用户不知道怎么使用。

@Winson-Huang
Copy link
Contributor Author

Winson-Huang commented Aug 14, 2023

所有可能的选项组合数量太多,没必要全部在 ProfilerCommand 类的 @Description 中列出。如果只是为新增的单个选项添加说明,直接使用 --help 选项就可以,如图所示:

图片

@Winson-Huang
Copy link
Contributor Author

Winson-Huang commented Aug 14, 2023

ProfilerCommand 类级别的 @Description 中目前的说明与改动后的行为基本是一致的,只有 format 选项的说明需要增加新支持的格式。目前新增的选项绝大部分都遵循了与 async-profiler 行为的一致性,若希望为用户提供更详细的信息,可以在 Arthas 文档或者ProfilerCommand 类级别的 @Description 中添加 async-profiler 的 README 文档和 Github Discussions 的链接。

@hengyunabc
Copy link
Collaborator

Add other output content formats that async-profiler supports to Arthas
profiler command, such as flat, traces, tree, etc.
Formerly, format option was used to control content format indirectly
through filename extension. Now, format option will directly control
content format, and it will still be used to generate output filename
extension when filename is not specified.
To keep consistent with async-profiler, add short name -I to --include option,
and short name -X to --exclude option.
@Winson-Huang
Copy link
Contributor Author

已在文档中更新了一些说明。

@WangJi92
Copy link
Contributor

已在文档中更新了一些说明。

文档最好增加一个命令特性的使用例子,极具代表性的,方便用户不需要去很深入的研究 profile 官方的文档,即可体验功能。

@Winson-Huang
Copy link
Contributor Author

已在文档添加示例。

@Winson-Huang
Copy link
Contributor Author

这个 PR 还需要进一步 review 吗?


默认情况下,结果文件是`html`格式,也可以用`--format`参数指定:
默认情况下,结果是 [Flame Graph](https://github.com/BrendanGregg/FlameGraph) 格式的 `html` 文件,也可以用 `-o` 或 `--format` 参数指定其他内容格式,包括 flat、traces、collapsed、flamegraph、tree、jfr。
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些新加的 format 都测试支持不?我之前删掉了一些,只保留 html ,因为好像只支持 html 了。

Copy link
Contributor Author

@Winson-Huang Winson-Huang Aug 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

正如我在最开始的说明中提到的,在本改动以前,arthas 中没有 -o 选项,仅有一个相关的 format 选项,它仅用于构造默认文件名的后缀(format 的值并未传递给 execute 方法)且只有两个可选值,【分析结果】的内容格式是让 async-profiler 通过这个后缀名推断的。

在本改动之后,format 选项更改为与 async-profiler 的 -o 选项完全对应,也就是仅控制分析结果的内容格式,与文件后缀名在一定程度上解耦合,当不明确指定输出文件名时,文件名后缀将通过 -o 选项的各种情况生成合适的值,或者使用默认值 html。具体的逻辑细节,可以查看代码中的改动。

经测试,指定这些格式后,当不明确指定文件名时,文件名后缀均可正确推断出来,且文件内容的格式也正确。

Comment on lines 589 to +624
private String outputFile() throws IOException {
if (this.file == null) {
String fileExt = outputFileExt();
File outputPath = ArthasBootstrap.getInstance().getOutputPath();
if (outputPath != null) {
this.file = new File(outputPath,
new SimpleDateFormat("yyyyMMdd-HHmmss").format(new Date()) + "." + this.format)
new SimpleDateFormat("yyyyMMdd-HHmmss").format(new Date()) + "." + fileExt)
.getAbsolutePath();
} else {
this.file = File.createTempFile("arthas-output", "." + this.format).getAbsolutePath();
this.file = File.createTempFile("arthas-output", "." + fileExt).getAbsolutePath();
}
}
return file;
}

/**
* This method should only be called when {@code this.file == null} is true.
*/
private String outputFileExt() {
String fileExt = "";
if (this.format == null) {
fileExt = "html";
} else if (this.format.startsWith("flat") || this.format.startsWith("traces")
|| this.format.equals("collapsed")) {
fileExt = "txt";
} else if (this.format.equals("flamegraph") || this.format.equals("tree")) {
fileExt = "html";
} else if (this.format.equals("jfr")) {
fileExt = "jfr";
} else {
// illegal -o option makes async-profiler use flat
fileExt = "txt";
}
return fileExt;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

format 选项相关的关键修改在这两个方法中。

@hengyunabc hengyunabc merged commit f375238 into alibaba:master Aug 18, 2023
12 checks passed
@hengyunabc hengyunabc added this to the 4.0.0 milestone Aug 18, 2023
@Winson-Huang Winson-Huang deleted the UpgradeActionStop branch August 18, 2023 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants